Ingest and import Windows Forms analyzer props #4605
Conversation
ghost
added
the
ghost
requested review from
packaging/Microsoft.NET.Sdk.WindowsDesktop/Microsoft.NET.Sdk.WindowsDesktop.ArchNeutral.csproj
Outdated
Show resolved
Hide resolved
RussKie
force-pushed
the
dev/igveliko/winforms_analyzer_props
branch
from
6d46dd9 to
443a26e
Compare
Provide a mechanism to import Windows Forms specific analyzers from Windows Forms transport package, and and package those into Microsoft.WindowsDesktop.App.Ref bundle, so that they are available in Windows Forms app out of the box. The analyzers are referenced via props and targets located in `C:\Program Files\dotnet\sdk\6.0.<version>\Sdks\Microsoft.NET.Sdk.WindowsDesktop\targets\`, which is facilitated by dotnet/wpf#4605. This work relates to dotnet/designs#223, and acts as a workaround until dotnet/designs#181 is approved and implemented.
| </ItemGroup> | ||
|
|
||
| <Target Name="_EnsureWindowsFormsPackagingContent" BeforeTargets="IdentifyPackageAssets"> |
There was a problem hiding this comment.
This is an antipattern. Foo.*ArchNeutral.csproj, Foo.csproj etc. have certain conventions, and they weren't intended to be spots for additional domain-specific targets. Our domain-specific targets - in fact all of this repos' build-related intelligence - lives under a certain hierarchy under eng\ that I've already alluded to. There are rare well-considered exceptions to this rule - and this does [edit] not [/edit] strike me as one.
There was a problem hiding this comment.
I haven't read the docs you've shared, but my sense/guess here is that analyzer related props/targets is a new category of payload that's being introduced. If this is right... then consider introducing a new Item name, something like AnalyzerPackagingContent that's used to track these items. These would become agnostic of WPF/Winforms etc. far as build-logic goes.
Then in Packaging.targets, add logic specific to validation of these. For e.g., anything that's been added to AnalyzerPackagingContent - ensure that they exist, make sure they're added to PackagingContent etc.. Then all you'd do in *.WindowsDesktop.ArchNeutral.csproj is adding your props/targets to the newly introduced AnalyzerPackagingContent Item and let the backend targets do its work as before.
There was a problem hiding this comment.
I added this target purely to perform verifcations as you suggested earlier.
All work is done by these targets https://github.com/RussKie/winforms/blob/project_configuration/src/System.Windows.Forms.Analyzers/src/System.Windows.Forms.Analyzers.targets
There was a problem hiding this comment.
The reason I added this target in this project and not in Directory.Build.targets at other levels is these validations is only relevant to this project. I'm happy to move it out to a separate targets file under eng\ folder and reference it in this project.
There was a problem hiding this comment.
A different architecture is fine, but this project is not the right place for adding targets, imports etc. - if you need to extend or hook the build system, it has to be done elsewhere.
There was a problem hiding this comment.
Does 9c0757e look better?
Provide a mechanism to ingest Windows Forms specific analyzer props and targets into Microsoft.NET.Sdk.WindowsDesktop from Windows Forms transport package, and copy these files into `targets` folder of the SDK, so that the props files are resolved and imported when a developer builds a Windows Forms app. In the end the new props file will end up in a location similar to this: `C:\Program Files\dotnet\sdk\6.0.<version>\Sdks\Microsoft.NET.Sdk.WindowsDesktop\targets\`. This work relates to dotnet/designs#223, and acts as a workaround until dotnet/designs#181 is approved and implemented.
RussKie
force-pushed
the
dev/igveliko/winforms_analyzer_props
branch
from
3d07448 to
9c0757e
Compare
|
Tested using an experimental pipeline: |
Description
This work relates to Streamline Windows Forms application configuration and bootstrap design proposal, and acts as a workaround until Proposal to add Framework specific inbox source generators design proposal is approved and implemented.
Provide a mechanism to ingest Windows Forms specific analyzer props into Microsoft.NET.Sdk.WindowsDesktop from Windows Forms transport package, and copy the props file into
targetsfolder of the SDK, so that the props file is resolved and imported when a developer builds a Windows Forms app.In the end the new props file will end up in a location similar to this:
C:\Program Files\dotnet\sdk\6.0.<version>\Sdks\Microsoft.NET.Sdk.WindowsDesktop\targets\.The build process looks something like this:
Testing
A lot of manual testing
winforms> .\build.cmd -packwpf> .\build -pack